Skip to content

fix: stop XHR retries once the request is aborted - #470

Open
igoroctaviano wants to merge 1 commit into
masterfrom
fix/xhr-retry-stop-on-abort
Open

igoroctaviano wants to merge 1 commit into
masterfrom
fix/xhr-retry-stop-on-abort

Conversation

@igoroctaviano

@igoroctaviano igoroctaviano commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Why

getXHRRetryHook schedules a re-send after a retryable status (429/500/502/503/504). If the caller cancels the request while a retry is waiting, the timer still fires: it calls request.open() and send() again and downloads data nobody needs. This matters for tile loading, where Viv/deck.gl aborts tiles that scroll out of view while panning. The hook had no way to know about the abort.

Related: ImagingDataCommons/dicom-microscopy-viewer#312 and ImagingDataCommons/dicom-microscopy-viewer#313 (headless tile reader with AbortSignal support).

What

dicomweb-client is getting a request.signal option. When it is set, request hooks receive the signal as metadata.signal. The retry hook now honors that signal:

  • a request whose signal is already aborted is not sent;
  • aborting while a retry is scheduled cancels it (operation.stop()), so there's no re-open and no re-send;
  • a retryable response that arrives after the abort is passed on to the client handler instead of being retried;
  • the abort listener is removed once the request finishes.

Without a signal, the hook behaves exactly as before. signal?: AbortSignal is added to DICOMwebClientRequestHookMetadata in AppConfig.d.ts.

Merging is safe on its own. It takes effect once the dicomweb-client version with request.signal is released and Slim uses it (dcmjs-org/dicomweb-client#144). With today's dicomweb-client, metadata.signal is never set, so nothing changes.

Testing

  • The existing 7 tests in src/utils/__tests__/xhrRetryHook.test.ts are unchanged and passing.

  • 4 new tests cover:

    • abort while a retry is scheduled (no re-send, no re-open);
    • a response arriving after the abort (not retried, passed on once);
    • a signal that's already aborted (not sent);
    • listener cleanup.

    All 4 fail against the previous hook.

  • Full suite: 177 passed. tsc --noEmit and biome are clean.

dicomweb-client passes the request's AbortSignal to request hooks as
`metadata.signal` when the `request.signal` option is used. The retry
hook now honors it: an aborted request is not sent, a scheduled retry is
cancelled instead of re-opening and re-sending the request, and a
retryable response that arrives after the abort is passed on rather than
retried. Without a signal the hook behaves as before.

This keeps cancelled tile requests (e.g. Viv tiles pruned while panning)
from being downloaded again after a 429/5xx.
@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@deepsource-io

deepsource-io Bot commented Nov 3, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 35fa187...a05a5a8 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Nov 3, 2026 10:07p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.49.0

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit a05a5a8):

https://idc-external-006--pr470-fix-xhr-retry-stop-o-mm1n59ff.web.app

(expires Tue, 10 Nov 2026 22:11:47 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant